-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Autocomplete focus scroll #3067
Conversation
Removed vultr server and associated DNS entries |
@@ -281,6 +337,7 @@ export const SelectMultiple = (props: SelectMultipleProps) => { | |||
aria-live="polite" | |||
disableClearable | |||
disableCloseOnSelect | |||
disableListWrap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was also flagged by @ianjon3s as a usability issue - you can now no longer wrap around from final → first option when navigating by keyboard.
// Using a custom listbox has a breaking effect on the native scroll used by MUI's Autocomplete | ||
// Using a MutationObserver we can listen for MUI applying the focusVisible class | ||
// We then use this to indicate that we need to scroll the focused element into view |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like a pretty complex solution to a small UI problem. Tried a few alternatives first, but if there's anything obvious I'm missing here please let me know!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving from a usability POV, nice work!
8ca955e
to
478b9f2
Compare
Merged to #2993 as this will have another pair of eyes on it before going to |
What does this PR do?
Autocomplete
componentDemo
Screen.Recording.2024-04-29.at.11.47.59.mov